Skip to content

Update EmsdkPackageVersion to use version from dependency flow#128261

Merged
akoeplinger merged 3 commits into
mainfrom
akoeplinger-patch-2
May 20, 2026
Merged

Update EmsdkPackageVersion to use version from dependency flow#128261
akoeplinger merged 3 commits into
mainfrom
akoeplinger-patch-2

Conversation

@akoeplinger
Copy link
Copy Markdown
Member

@akoeplinger akoeplinger commented May 15, 2026

It got hardcoded a while ago to workaround a build problem.

@akoeplinger
Update EmsdkPackageVersion to use version from dependency flow
508a530 
It got hardcoded a while ago to workaround a build problem.
Copilot AI review requested due to automatic review settings May 15, 2026 16:44
@github-actions github-actions Bot added the area-codeflow for labeling automated codeflow label May 15, 2026
Copilot AI reviewed May 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the build’s Emscripten SDK provisioning versioning so it no longer relies on a hardcoded EmsdkPackageVersion value in eng/Versions.props, instead sourcing it from a dependency-flow-updated property.

Changes:

  • Replace the hardcoded EmsdkPackageVersion (10.0.0) with $(MicrosoftDotNetApiCompatTaskPackageVersion).
  • Add an explanatory comment about why a stable, dependency-flow-backed version property is used.

Comment thread eng/Versions.props Outdated
@github-actions

This comment has been minimized.

Sign in to view
This was referenced May 16, 2026
Open
Open
Copilot AI review requested due to automatic review settings May 18, 2026 13:20
Copilot AI reviewed May 18, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread eng/Versions.props
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

Caution

Security scanning requires review for Code Review

Details

The threat detection results could not be parsed. The workflow output should be reviewed before merging.

Review the workflow run logs for details.

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #128261

Holistic Assessment

Motivation: Justified — removing a hardcoded version (10.0.0) that was a workaround and replacing it with the dependency-flowed version eliminates manual bump maintenance.

Approach: Correct — uses $(MicrosoftNETCoreAppRefPackageVersion) (currently 11.0.0-preview.5.26257.113) which provides runtime version numbers matching the emsdk package ID pattern. This mirrors how MicrosoftNETRuntimeEmscriptenVersion already uses a flowed version on the adjacent line.

Summary: ✅ LGTM. Single-property change in eng/Versions.props, aligning EmsdkPackageVersion with dependency flow. The consumers in eng/AcquireEmscriptenSdk.targets and src/mono/mono.proj will now automatically get the correct version without manual updates.

Detailed Findings

✅ Correctness — Version source is appropriate

MicrosoftNETCoreAppRefPackageVersion (defined in eng/Version.Details.props) provides runtime-style version numbers (e.g., 11.0.0-preview.5.26257.113), which matches the versioning scheme used by the emsdk NuGet packages (Microsoft.NET.Runtime.Emscripten.{EmsdkVersion}.{component}). The comment added in the PR correctly explains the rationale.

✅ Consistency — Follows established pattern

The adjacent MicrosoftNETRuntimeEmscriptenVersion already uses a flowed version ($(MicrosoftDotNetApiCompatTaskPackageVersion)) for the same reason. This PR makes EmsdkPackageVersion consistent with that approach.

✅ Scope — Minimal and focused

Only one property value changed, with appropriate explanatory comments added. No risk of unintended side effects.

Generated by Code Review for issue #128261 · ● 1.7M · ◷

This was referenced May 18, 2026
Open
Open
Open
@akoeplinger
Copy link
Copy Markdown
Member Author

akoeplinger commented May 20, 2026

/ba-g known issues (BA didn't match it due to build timeouts)

@akoeplinger akoeplinger merged commit 58f1b03 into main May 20, 2026
173 of 177 checks passed
@akoeplinger akoeplinger deleted the akoeplinger-patch-2 branch May 20, 2026 16:27
@dotnet-maestro dotnet-maestro Bot mentioned this pull request May 21, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@lewing lewing lewing approved these changes

Assignees

@akoeplinger akoeplinger

Labels

area-codeflow for labeling automated codeflow

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@akoeplinger @lewing